Extract rule: template-self-closing-void-elements#2617
Conversation
NullVoxPopuli-ai-agent
left a comment
There was a problem hiding this comment.
Review: template-self-closing-void-elements (PR #2617)
Comparison with ember-template-lint source
General correctness:
-
Fix implementation is incomplete: The rule declares
fixable: 'code'in meta, but thecreate()function never provides afixfunction incontext.report(). This means ESLint will advertise the rule as fixable, but--fixwill not actually change anything. The tests useoutput: nullconfirming no fix is applied. Either removefixable: 'code'from meta, or implement the fixer. The original ETL rule does implement fixing (usingember-template-recastbuilders andthis.mode === 'fix'). -
Detection mechanism differs: The original ETL checks
this.sourceForNode(node).trim()and looks at the last two characters (/>or not). The ESLint version usesnode.selfClosingproperty. This is cleaner and should be reliable if the Glimmer ESLint parser correctly setsselfClosingon element nodes. Worth verifying that the parser'sselfClosingmatches the source text behavior. -
Void element list: Both versions use the same set of 16 void elements. The ESLint version uses a
Set(better lookup performance), the original uses a plain object. Functionally equivalent. -
Schema: The ESLint version uses
oneOf: [{ type: 'boolean' }, { type: 'string', enum: ['require'] }]. The original ETL acceptstrue(enable default),false(disable), or'require'. In the ESLint version, passingtrueorfalsewould be handled —trueis truthy but not=== 'require', so it falls into the "disallow self-closing" default behavior.falsesimilarly falls into default. This differs from the original wherefalsedisables the rule entirely. Bug: Whenoptions[0]isfalse, the ESLint version still runs and reports self-closing elements as redundant. It should return early/disable whenfalseis passed. -
Default behavior when no config: When no option is passed,
configisundefined, andrequireSelfClosingis false, so the rule defaults to disallowing self-closing. This matches the original ETL's default behavior (configtrue→ disallow self-closing).
Scope analysis (gjs/gts):
Not applicable. This rule checks HTML void element tags and self-closing syntax — purely structural, no name matching involved.
Tests: Good coverage of all 16 void elements in both modes, including the 'require' config variant. The complex void element test with attributes, modifiers, comments, and block params is a nice addition.
Summary: Two issues to address: (1) fixable: 'code' is declared but no fixer is implemented — either implement it or remove the declaration; (2) false config should disable the rule rather than running with default behavior.
🤖 Automated review comparing with ember-template-lint source
2f60fb2 to
af0a016
Compare
Split from #2371.